Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #2096 update rtems toolchain files #2097

Closed
wants to merge 1 commit into from
Closed

Fix #2096 update rtems toolchain files #2097

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 27, 2022

Checklist (Please check before submitting)

Describe the contribution

Modifies the existing RTEMS 4.11 and RTEMS 5 toolchain files to:

  • Change the OS define to avoid using a reserved identifier (RTEMS(x)_)
  • Change the RTEMS 4.11 define to indicate it is deprecated
  • Add a BSP_SPECS_FLAGS flag to allow for RTEMS version variations in the specs flags
  • Add a dynamic load Cmake variable to allow for two types of CMAKE executable types in the platform file

Added a RTEMS 6 i686 toolchain file

Testing performed

  • Rebuilt and ran RTEMS 4.11 and RTEMS 5 tests
  • Tested RTEMS 6 build and ran tests (using local OSAL mods and CI docker image with RTEMS 6)

Expected behavior changes

  • No impact or behavior changes on RTEMS 4.11 and RTEMS 5

System(s) tested on

  • RTEMS 4.11, RTEMS 5 Docker images used in CI workflows

Additional context

  • The changes in the toolchain files for RTEMS 4.11 and RTEMS 5 are to accommodate the RTEMS 6 port and allow for additional RTEMS builds to use the same CMake Platform file for RTEMS.

Contributor Info - All information REQUIRED for consideration of pull request
Alan Cudmore, NASA/GSFC Code 582.0

@ghost ghost requested a review from jphickey April 27, 2022 13:25
@astrogeco
Copy link
Contributor

astrogeco commented Apr 27, 2022

CCB:2022-04-27

  • Edit commits to follow standard
  • Does this have dependencies in other repos?
  • Consider a consolidating into a common #include and add the differences
  • Should end up moving to always building as a dynamic module

@ghost
Copy link
Author

ghost commented Apr 28, 2022

Addressing the CCB comments:
Edit commits to follow standard:
I left the # off of the 2nd commit message. Is that the problem with the commit?
Dependencies on other repos?
No - this does not depend on any other commits that I am making
Consider consolidating into a common CMake include for RTEMS toolchain files:
I took a look at this and there are only a handful of common lines in the files. I re-arranged the toolchain files in my latest commit so the common statements are at the bottom. Please let me know if you think it's worth factoring out those statements and I can make another commit. If not, these should work - I have built RTEMS 4.11, 5, and 6

@astrogeco
Copy link
Contributor

astrogeco commented May 4, 2022

Just a nit, an you amend the commit to switch uppercase "FIX" to regular "Fix"?

@ghost ghost marked this pull request as ready for review May 4, 2022 15:15
@ghost ghost added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label May 4, 2022
@astrogeco
Copy link
Contributor

astrogeco commented May 4, 2022

CCB:2022-05-04, APPROVED with CHANGES

@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels May 4, 2022
@ghost
Copy link
Author

ghost commented May 4, 2022

CCB suggestion:
Remove: OSAL_SYSTEM_BSPTYPE and OSAL_SYSTEM_OSTYPE from toolchain files, because these are now setup in the PSP.

Question:
A couple of PSPs have the following line in the build_options.cmake file:
set(CFE_PSP_EXPECTED_OSAL_BSPTYPE "generic-vxworks")
But the pc-rtems file does not. Is this needed before removing the OSAL_SYSTEM_BSPTYPE in the toolchain file?
(Edit: I removed the OSAL_SYSTEM_BSPTYPE and OSAL_SYSTEM_OSTYPE and the CFE_PSP_EXPECTED_OSAL_BSPTYPE did not seem to be required)

@ghost
Copy link
Author

ghost commented May 4, 2022

  • I removed OSAL_SYSTEM_BSPTYPE and OSAL_SYSTEM_OSTYPE as they are not needed any more.
  • Rebased/Squashed to 1 commit.
  • Tested build of i686-rtems5 (built and ran bundle then all CI tests)
  • Tested build of i686-rtems4.11 (built and ran bundle then all CI tests)

I believe this is ready to merge

@ghost ghost mentioned this pull request May 6, 2022
2 tasks
@astrogeco astrogeco changed the title Fix 2096 update rtems toolchain files Fix #2096 update rtems toolchain files May 12, 2022
astrogeco added a commit that referenced this pull request May 17, 2022
astrogeco added a commit to nasa/cFS that referenced this pull request May 18, 2022
Combines

nasa/cFE#2098, v7.0.0-rc4+dev127
nasa/osal#1251, v6.0.0-rc4+dev74
nasa/PSP#343, v1.6.0-rc4+dev34

**cFE**

nasa/cFE#2078, Adds truncation warning suppression flags

nasa/cFE#2094, Doc deploy from local workflow on main-branch push

nasa/cFE#2088, Add support for fractional seconds in epoch

nasa/cFE#2044, Remove redundant word in App Developers Guide

nasa/cFE#2099,  UT updates for alternate time configuration

**RTEMS 6 Support

nasa/cFE#2097, update rtems toolchain files

nasa/osal#1250, add os-impl-no-select.c for RTEMS

nasa/PSP#339, Update RTEMS CMake Platform File

**PSP**

nasa/PSP#341, Add cpu affinity example

**Fix Broken CodeQL Workflow Reference**

nasa/PSP#337

nasa/osal#1249

nasa/sample_lib#82

nasa/sample_app#173

nasa/sch_lab#113

nasa/ci_lab#109

nasa/to_lab#120

nasa/elf2cfetbl#110

nasa/tblCRCTool#68

nasa/cFS-GroundSystem#214

Co-authored-by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored-by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored-by: Jonathan Branderburg <jonathan-brandenburg-metecs@users.noreply.github.com>
Co-authored-by: Alan Cudmore <acudmore@users.noreply.github.com>
Co-authored-by: Hugo Valente <HugoValente11@users.noreply.github.com>
astrogeco added a commit to nasa/cFS that referenced this pull request May 18, 2022
Combines

nasa/cFE#2098, v7.0.0-rc4+dev127
nasa/osal#1251, v6.0.0-rc4+dev74
nasa/PSP#343, v1.6.0-rc4+dev34

**cFE**

nasa/cFE#2078, Adds truncation warning suppression flags

nasa/cFE#2094, Doc deploy from local workflow on main-branch push

nasa/cFE#2088, Add support for fractional seconds in epoch

nasa/cFE#2044, Remove redundant word in App Developers Guide

nasa/cFE#2099,  UT updates for alternate time configuration

**RTEMS 6 Support

nasa/cFE#2097, update rtems toolchain files

nasa/osal#1250, add os-impl-no-select.c for RTEMS

nasa/PSP#339, Update RTEMS CMake Platform File

**PSP**

nasa/PSP#341, Add cpu affinity example

**Fix Broken CodeQL Workflow Reference**

nasa/PSP#337

nasa/osal#1249

nasa/sample_lib#82

nasa/sample_app#173

nasa/sch_lab#113

nasa/ci_lab#109

nasa/to_lab#120

nasa/elf2cfetbl#110

nasa/tblCRCTool#68

nasa/cFS-GroundSystem#214

Co-authored-by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored-by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored-by: Jonathan Branderburg <jonathan-brandenburg-metecs@users.noreply.github.com>
Co-authored-by: Alan Cudmore <acudmore@users.noreply.github.com>
Co-authored-by: Hugo Valente <HugoValente11@users.noreply.github.com>
astrogeco added a commit to nasa/cFS that referenced this pull request May 19, 2022
Combines

nasa/cFE#2098, v7.0.0-rc4+dev127
nasa/osal#1251, v6.0.0-rc4+dev74
nasa/PSP#343, v1.6.0-rc4+dev34

**cFE**

nasa/cFE#2078, Adds truncation warning suppression flags

nasa/cFE#2094, Doc deploy from local workflow on main-branch push

nasa/cFE#2088, Add support for fractional seconds in epoch

nasa/cFE#2044, Remove redundant word in App Developers Guide

nasa/cFE#2099,  UT updates for alternate time configuration

**RTEMS 6 Support

nasa/cFE#2097, update rtems toolchain files

nasa/osal#1250, add os-impl-no-select.c for RTEMS

nasa/PSP#339, Update RTEMS CMake Platform File

**PSP**

nasa/PSP#341, Add cpu affinity example

**Fix Broken CodeQL Workflow Reference**

nasa/PSP#337

nasa/osal#1249

nasa/sample_lib#82

nasa/sample_app#173

nasa/sch_lab#113

nasa/ci_lab#109

nasa/to_lab#120

nasa/elf2cfetbl#110

nasa/tblCRCTool#68

nasa/cFS-GroundSystem#214

Co-authored-by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored-by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored-by: Jonathan Branderburg <jonathan-brandenburg-metecs@users.noreply.github.com>
Co-authored-by: Alan Cudmore <acudmore@users.noreply.github.com>
Co-authored-by: Hugo Valente <HugoValente11@users.noreply.github.com>
@astrogeco
Copy link
Contributor

Merged in 41cadde

@astrogeco astrogeco closed this May 19, 2022
@skliper skliper added this to the Draco milestone Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB draco-rc2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update RTEMS toolchain files for RTEMS 4.11, 5, and 6
3 participants